Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sap_swpm, sap_general_preconfigure: feat: add variables for sap_install FQCN collection name for calling roles #925

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

marcelmamula
Copy link
Contributor

@marcelmamula marcelmamula commented Jan 6, 2025

Description

  • Add variables for setting collection name to remove hardcode dependency.
  • Affected: sap_swpm and sap_general_preconfigure for calling sap_maintain_etc_hosts

Validation

Change was tested on SLES4SAP 15 SP6 on AWS with SAP ASCS/ERS HA build.

Note

@marcelmamula marcelmamula added the enhancement New feature or request label Jan 6, 2025
@marcelmamula marcelmamula self-assigned this Jan 6, 2025
Copy link
Member

@berndfinger berndfinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@berndfinger berndfinger merged commit bee8bbe into sap-linuxlab:dev Jan 7, 2025
4 of 5 checks passed
@sean-freeman sean-freeman changed the title sap_swpm, sap_general_preconfigure: feat: add variables for sap_install collection name for calling roles sap_swpm, sap_general_preconfigure: feat: add variables for sap_install FQCN collection name for calling roles Jan 7, 2025
@sean-freeman
Copy link
Member

sean-freeman commented Jan 7, 2025

Updated the GH Issue title slightly (for easier reference in future), and added to description the history (inc. specific commit where FQCN var was added previously).

Personally I only see the need for a variable that alters the namespace of the FQCN, I do not know how many people will want to fork (into new namespace) and then also deviate from the collection name "sap_install".

@berndfinger
Copy link
Member

Personally I only see the need for a variable that alters the namespace of the FQCN, I do not know how many people will want to fork (into new namespace) and then also deviate from the collection name "sap_install".

I had the same topic in mind while reviewing but as we already have a variable ..._system_roles_collection in some roles, I thought using the full collection name is better and easier to understand than something like sap_general_preconfigure_sap_install_namespace which we probably would have to explain in more detail.

Let's see if we will have other aspects to consider, and then change from collection to namespace.

@marcelmamula
Copy link
Contributor Author

@berndfinger @sean-freeman This was exact reason why I added it this way - same functionality as with *_system_roles_collection.

@marcelmamula marcelmamula deleted the fqcn branch January 8, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants